Skip to content

[Types] Complete ExtensionObject decoding and add an atomic file replace capability - #4128

Open
marcschier wants to merge 7 commits into
masterfrom
marcschier/wot-01-core-types
Open

[Types] Complete ExtensionObject decoding and add an atomic file replace capability#4128
marcschier wants to merge 7 commits into
masterfrom
marcschier/wot-01-core-types

Conversation

@marcschier

Copy link
Copy Markdown
Collaborator

Summary

This is PR 1 of a stacked split of integration PR #4093 into independently reviewable pieces. It has no dependency on any other PR in the stack and contains no WoT content.

Changes included here:

  • Complete ExtensionObject raw-body decoding for binary, XML, and JSON bodies by using the message context's encodeable factory to resolve the concrete type.
  • Simplify Variant structure helpers by delegating the decoding work to ExtensionObject.
  • Add IAtomicFileReplace as an optional IFileSystem capability so existing external IFileSystem implementations are not broken.
  • Implement atomic publish for LocalFileSystem with File.Replace or File.Move and for VirtualFileSystem by re-keying the in-memory entry.
  • Add direct LocalFileSystem and VirtualFileSystem tests for move, overwrite, consumed source, missing source, complete destination content, and capability assignability.

Validation

  • dotnet build tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -c Release -p:CustomTestTarget=net10.0 -v:m
  • dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -c Release -p:CustomTestTarget=net10.0 --no-build
  • dotnet build tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -c Release -p:CustomTestTarget=net48 -v:m
  • dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -c Release -p:CustomTestTarget=net48 --no-build

…bility

ExtensionObject.TryGetValue took an IServiceMessageContext but never used it:
the body contained a "TODO: Decode if possible" and returned false whenever the
payload was still raw. It now decodes binary, XML and JSON bodies through the
codecs, resolving the concrete type from the context's IEncodeableFactory, so no
parameterless constructor constraint is needed. Behaviour without a context is
unchanged.

Variant's structure helpers now delegate to that instead of carrying a second
implementation of the same logic.

IAtomicFileReplace is a new optional capability for IFileSystem implementations
that can publish a fully written file under its final name in one indivisible
step. It is deliberately a separate interface rather than a member on
IFileSystem, because that interface is implemented outside this repository and
the library targets frameworks without default interface members. LocalFileSystem
implements it with File.Replace or File.Move, and VirtualFileSystem by re-keying
its in-memory entry. Durable writers can then stage content under a temporary
name and publish it, so an interrupted write can never leave a partially written
file visible at the destination.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
Copilot AI review requested due to automatic review settings July 30, 2026 17:22
Comment thread src/Opc.Ua.Types/Utils/FileSystem/IAtomicFileReplace.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds full ExtensionObject raw-body decoding (binary/XML/JSON) and introduces an optional atomic file replace capability for file systems, along with targeted tests.

Changes:

  • Implement raw-body decoding in ExtensionObject and update Variant helpers to delegate structure decoding to it.
  • Introduce IAtomicFileReplace and implement atomic publish for LocalFileSystem and VirtualFileSystem.
  • Add/extend tests covering atomic replace behaviors and updated type/variant/extensionobject semantics.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Opc.Ua.Types.Tests/Utils/FileSystem/AtomicFileReplaceTests.cs New tests validating atomic replace semantics for local and virtual file systems.
tests/Opc.Ua.Types.Tests/Nodes/TypeTableTests.cs Adds coverage for IsEncodingFor and IsTypeOf equivalence scenarios.
tests/Opc.Ua.Types.Tests/BuiltIn/VariantCoverageTests.cs Adds coverage for Variant.TryGetStructure(..., context, ...) decoding behavior.
tests/Opc.Ua.Types.Tests/BuiltIn/ExtensionObjectTests.cs Adds coverage for ExtensionObject decoding of binary/XML/JSON raw bodies.
src/Opc.Ua.Types/Utils/FileSystem/VirtualFileSystem.cs Implements IAtomicFileReplace.Replace via in-memory entry “re-keying”.
src/Opc.Ua.Types/Utils/FileSystem/LocalFileSystem.cs Implements IAtomicFileReplace.Replace using File.Replace / File.Move.
src/Opc.Ua.Types/Utils/FileSystem/IAtomicFileReplace.cs New capability interface documenting atomic publish semantics.
src/Opc.Ua.Types/Nodes/TypeTable.cs Makes IsTypeOf reflexive and relaxes IsEncodingFor to accept direct DataTypeIds.
src/Opc.Ua.Types/BuiltIn/Variant.cs Adds context-aware structure extraction overloads + documentation improvements.
src/Opc.Ua.Types/BuiltIn/TypeInfo.cs Improves ExtensionObject datatype detection when TypeId is already a known datatype.
src/Opc.Ua.Types/BuiltIn/ExtensionObject.cs Adds raw-body decoding using the message context encodeable factory.

Comment thread src/Opc.Ua.Types/Utils/FileSystem/VirtualFileSystem.cs
Comment thread src/Opc.Ua.Types/Utils/FileSystem/LocalFileSystem.cs
Comment thread src/Opc.Ua.Types/BuiltIn/ExtensionObject.cs
Replace the separate optional IAtomicFileReplace capability with a Replace member on IFileSystem at the maintainer's request, and implement the member on all in-repo file system implementations.

Update VirtualFileSystem.Replace to use a TryGetValue/TryUpdate/TryAdd retry loop so the dictionary update is conditional and exactly the replaced entry is disposed after a successful update.

Use File.Move with overwrite on modern TFMs for LocalFileSystem.Replace so overwrites work on platforms where File.Replace is unavailable, while keeping the File.Replace fallback for net48.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.54545% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.92%. Comparing base (cfd15f0) to head (d7caecc).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Types/BuiltIn/ExtensionObject.cs 72.72% 13 Missing and 8 partials ⚠️
src/Opc.Ua.Types/BuiltIn/Variant.cs 91.66% 1 Missing and 1 partial ⚠️
...c/Opc.Ua.Types/Utils/FileSystem/LocalFileSystem.cs 60.00% 1 Missing and 1 partial ⚠️
src/Opc.Ua.Types/BuiltIn/TypeInfo.cs 80.00% 0 Missing and 1 partial ⚠️
...c.Ua.SourceGeneration/SourceGeneratorFileSystem.cs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4128      +/-   ##
==========================================
- Coverage   80.18%   79.92%   -0.27%     
==========================================
  Files        1514     1515       +1     
  Lines      209245   210114     +869     
  Branches    36008    36244     +236     
==========================================
+ Hits       167789   167932     +143     
- Misses      28878    29573     +695     
- Partials    12578    12609      +31     
Files with missing lines Coverage Δ
src/Opc.Ua.Types/Nodes/TypeTable.cs 98.57% <100.00%> (+0.02%) ⬆️
...pc.Ua.Types/Utils/FileSystem/CombinedFileSystem.cs 55.55% <100.00%> (+4.04%) ⬆️
...rc/Opc.Ua.Types/Utils/FileSystem/NullFileSystem.cs 100.00% <100.00%> (ø)
...pc.Ua.Types/Utils/FileSystem/ResourceFileSystem.cs 40.00% <100.00%> (+1.76%) ⬆️
...Opc.Ua.Types/Utils/FileSystem/VirtualFileSystem.cs 87.58% <100.00%> (+0.86%) ⬆️
src/Opc.Ua.Types/BuiltIn/TypeInfo.cs 83.35% <80.00%> (+0.86%) ⬆️
...c.Ua.SourceGeneration/SourceGeneratorFileSystem.cs 44.82% <0.00%> (-1.61%) ⬇️
src/Opc.Ua.Types/BuiltIn/Variant.cs 80.78% <91.66%> (+0.24%) ⬆️
...c/Opc.Ua.Types/Utils/FileSystem/LocalFileSystem.cs 20.00% <60.00%> (+13.33%) ⬆️
src/Opc.Ua.Types/BuiltIn/ExtensionObject.cs 78.54% <72.72%> (-2.41%) ⬇️

... and 91 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Opc.Ua.Types/BuiltIn/TypeInfo.cs
Explain why the ExtensionObject TypeId path checks known datatype ids before falling back to encoding lookup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
@marcschier marcschier added the ready Ready to merge once CI Passes label Aug 3, 2026
PubSubApplication.StartAsync used to log connection EnableAsync failures and continue, leaving the application marked started even though the failed connection had no transport. Discovery and Action callers then saw misleading invalid-operation errors instead of the underlying transport failure. Re-throw enable failures after stopping the partially-started application so environment failures surface at startup.

A channel-manager lease swapped from a terminal entry to an already-open fresh entry missed the fresh entry's current state transition because the lease was not attached while OpenInitialAsync moved the entry to Ready. ReattachParticipant now replays the fresh entry's current observable state to the lease, so ManagedSession.ChannelStateChanged consumers see Ready after recovery.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
Handle the terminal reconnect race where an entry faults after the caller has captured it but before or while RequestReconnectAsync observes the state. Dispose transports that finish opening after an entry has already been closed so swapped leases cannot keep stale certificate/channel handles alive.

For the transfer subscription fixture, prefer an actual Basic256Sha256 Sign endpoint when the test asks for a secure session instead of mutating whichever endpoint happens to be first. This keeps the secure-channel certificate and policy consistent across platforms.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants